-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(node): add paramters to forward OTEL Headers to Clickhouse #395
base: main
Are you sure you want to change the base?
Conversation
Thanks for the contribution. |
@slvrtrn that makes sense, but what about the existing auth configuration parameters that get turned into headers? Should custom headers override them, or visa versa? |
Also I had a look at the failing integration tests it seems that the CI environment is misconfigured? |
👍
I'd vote for allowing users to override any headers (with a debug log record emitted that an override has happened) - it gives more power and flexibility to the end user. For example, one can implement multi-tenant runtime and mange users on the app level |
Let's make the same logic to the default headers in the connection class: https://github.com/ClickHouse/clickhouse-js/blob/main/packages/client-node/src/connection/node_base_connection.ts#L97-L101 (+ e.g. can override all except
Yep, it cannot run the tests with a Cloud instance if a PR is created by an external contributor. |
Summary
Addresses #394, Clickhouse supports reading the OTEL HTTP headers from the connection
Checklist